-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Range Iterators #50284
Merged
Merged
Implement Range Iterators #50284
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Should this also be implemented in LocalVector, Array and Dictionary? |
timothyqiu
reviewed
Jul 8, 2021
@Calinou possibly, but I would wait since those need to be converted different. |
reduz
force-pushed
the
implement-range-iterators
branch
from
July 8, 2021 15:56
829db94
to
03f7aa5
Compare
timothyqiu
reviewed
Jul 8, 2021
JFonS
reviewed
Jul 8, 2021
reduz
force-pushed
the
implement-range-iterators
branch
from
July 8, 2021 20:49
03f7aa5
to
e949ee7
Compare
JFonS
approved these changes
Jul 8, 2021
timothyqiu
reviewed
Jul 9, 2021
reduz
force-pushed
the
implement-range-iterators
branch
from
July 9, 2021 02:26
e949ee7
to
efd92e8
Compare
This PR implements range iterators in the base containers (Vector, Map, List, Pair Set). Given several of these data structures will be replaced by more efficient versions, having a common iterator API will make this simpler. Iterating can be done as follows (examples): ```C++ //Vector<String> for(const String& I: vector) { } //List<String> for(const String& I: list) { } //Map<String,int> for(const KeyValue<String,int>&I : map) { print_line("key: "+I.key+" value: "+itos(I.value)); } //if intending to write the elements, reference can be used //Map<String,int> for(KeyValue<String,int>& I: map) { I.value = 25; //this will fail because key is always const //I.key = "hello" } ``` The containers are (for now) not STL compatible, since this would mean changing how they work internally (STL uses a special head/tail allocation for end(), while Godot Map/Set/List do not). The idea is to change the Godot versions to be more compatible with STL, but this will happen after conversion to new iterators have taken place.
reduz
force-pushed
the
implement-range-iterators
branch
from
July 9, 2021 02:27
efd92e8
to
a9c943b
Compare
akien-mga
reviewed
Jul 9, 2021
Thanks! |
With: // Also fails with:
// const Vector<Vector3> vertices = arrays[Mesh::ARRAY_VERTEX];
const PackedVector3Array vertices = arrays[Mesh::ARRAY_VERTEX];
for (const Vector3& vertex : vertices) { // Same error with Vector3 (without `&`).
st->add_vertex(vertex);
} I get the following compilation error with Clang 12:
|
This was referenced Aug 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements range iterators in the base containers (Vector, Map, List, Pair Set).
Given several of these data structures will be replaced by more efficient versions, having a common iterator API will make this simpler.
Iterating can now be done as follows (examples):
The containers are (for now) not STL compatible, since this would mean changing how they work internally (STL uses a special head/tail allocation for end(), while Godot Map/Set/List do not).
The idea is to change the Godot versions to be more compatible with STL, but this will happen after conversion to new iterators have taken place.
Note that for better code readability (specially when we are reviewing PRs), auto is still banned. Iterators must be defined with the right type.
After this Pull Request is merged, we will be looking for contributors to lend a hand converting the codebase to use this new syntax as much as possible.
Some rules for the conversion work: